Skip to content

refactor: remove redundant --disable-workload-identity#4871

Merged
zhoxing-ms merged 4 commits intoAzure:mainfrom
bcho:aks/remove-disable-workload-identity-flag
May 24, 2022
Merged

refactor: remove redundant --disable-workload-identity#4871
zhoxing-ms merged 4 commits intoAzure:mainfrom
bcho:aks/remove-disable-workload-identity-flag

Conversation

@bcho
Copy link
Member

@bcho bcho commented May 23, 2022 •
edited
Loading

This pull request removed the redundant --disable-workload-identity flag from az aks create|update commands. Existing user can use --enable-workload-identity False to archive such functionality.

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az aks create

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

@bcho bcho force-pushed the aks/remove-disable-workload-identity-flag branch from 6e48772 to 8f48eea Compare May 23, 2022 03:40
@yonzhan
Copy link
Collaborator

yonzhan commented May 23, 2022

AKS

@yonzhan yonzhan added this to the Jun 2022 (2022-07-05) milestone May 23, 2022
@bcho bcho changed the title [WIP] refactor: remove redundant --disable-workload-identity refactor: remove redundant --disable-workload-identity May 23, 2022
FumingZhang
FumingZhang approved these changes May 23, 2022
View reviewed changes
Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

bingosummer
bingosummer suggested changes May 23, 2022
View reviewed changes
@yonzhan
Copy link
Collaborator

yonzhan commented May 23, 2022

Please resolve CI conflicting files first.

@bcho
Copy link
Member Author

bcho commented May 23, 2022

Please resolve CI conflicting files first.

Updated, PTAL @yonzhan

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented May 23, 2022

Will removing parameter --disable-workload-identity directly have a great impact on users?
If so, can we consider using c.deprecate(hide=True) to hide this parameter first? When most users migrate to parameter --enable-workload-identity False, then we remove it completely, so as to avoid big breaking change.

@bcho
Copy link
Member Author

bcho commented May 23, 2022

Will removing parameter --disable-workload-identity directly have a great impact on users? If so, can we consider using c.deprecate(hide=True) to hide this parameter first? When most users migrate to parameter --enable-workload-identity False, then we remove it completely, so as to avoid big breaking change.

@zhoxing-ms

Thanks for the concern. Removing these flags won't impact current user as in AKS, this feature is not being enabled yet, only a small group of testers have access to it. So, removing it won't cause breaking change.

@bcho
Copy link
Member Author

bcho commented May 24, 2022

@zhoxing-ms @yonzhan could you help merge this pr? Thanks!

@zhoxing-ms zhoxing-ms merged commit cba76d9 into Azure:main May 24, 2022
@bcho bcho deleted the aks/remove-disable-workload-identity-flag branch May 24, 2022 04:07
@FumingZhang FumingZhang mentioned this pull request Jun 14, 2022
Merged
3 tasks
FumingZhang added a commit to FumingZhang/azure-cli-extensions that referenced this pull request Jun 14, 2022
@FumingZhang
sync workload identity, PR Azure#4871
fab3641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhoxing-ms zhoxing-ms zhoxing-ms approved these changes

@FumingZhang FumingZhang FumingZhang approved these changes

@andyliuliming andyliuliming Awaiting requested review from andyliuliming andyliuliming is a code owner

@wangzelin007 wangzelin007 Awaiting requested review from wangzelin007

+1 more reviewer

@bingosummer bingosummer bingosummer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@zhoxing-ms zhoxing-ms

Labels

None yet

Projects

None yet

Milestone

Jun 2022 (2022-07-05)

Development

Successfully merging this pull request may close these issues.

5 participants

@bcho @yonzhan @zhoxing-ms @bingosummer @FumingZhang